Skip to content

fix: NetworkAnimator rising RTT issue#2236

Merged
NoelStephensUnity merged 8 commits intodevelopfrom
fix/networkanimator-rising-rtt
Oct 5, 2022
Merged

fix: NetworkAnimator rising RTT issue#2236
NoelStephensUnity merged 8 commits intodevelopfrom
fix/networkanimator-rising-rtt

Conversation

@NoelStephensUnity
Copy link
Copy Markdown
Member

@NoelStephensUnity NoelStephensUnity commented Oct 5, 2022

This fixes the issue with the rising RTT values discovered when testing the develop branch with BossRoom and now the NetworkAnimator.CheckForAnimatorChanges is no longer adding to the list but pulling from it, modifying, and then applying the modified AnimationState back to the same index position like the NetworkAnimator.ServerSynchronizeNewPlayer method was doing. This includes a better approach to handling updating dirty states.

The testing for this is a bit tricky, but I did add some modifications to the trigger test where it makes sure the AnimationStates list within the AnimationMessage was not increasing in size over time (a part of the RTT bug was it would add as opposed to apply the state for each AnimationState change detected).

MTT-4803

Testing and Documentation

  • Includes integration test update.

This fixes the issue where NetworkAnimator was not completely updated to the changes with AnimationMessage.  NetworkAnimator will now only send states that have changed.
adding additional checks to assure or AnimationStates list does not increase in size over time.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review October 5, 2022 00:41
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner October 5, 2022 00:41
Comment thread com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
Comment thread com.unity.netcode.gameobjects/Components/NetworkAnimator.cs Outdated
Comment thread com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
NoelStephensUnity and others added 4 commits October 5, 2022 09:22
Removed the check for AnimationMessage.AnimationStates being null as it will always be null when deserializing.
Updated several comments for readability and clarity.
noticed the local var was no longer a valid name.
renaming animMsg to animationState.
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) October 5, 2022 18:54
@NoelStephensUnity NoelStephensUnity merged commit e9d9454 into develop Oct 5, 2022
@NoelStephensUnity NoelStephensUnity deleted the fix/networkanimator-rising-rtt branch October 5, 2022 20:32
jakobbbb pushed a commit to GooseGirlGames/com.unity.netcode.gameobjects that referenced this pull request Feb 22, 2023
* fix
This fixes the issue where NetworkAnimator was not completely updated to the changes with AnimationMessage.  NetworkAnimator will now only send states that have changed.
Removed the check for AnimationMessage.AnimationStates being null as it will always be null when deserializing.
Updated several comments for readability and clarity.

* test
adding additional checks to assure or AnimationStates list does not increase in size over time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants